passthrough/stubdom: clean up hypercall privilege checking

This patch adds securty checks for pci passthrough related hypercalls
to enforce that the current domain owns the resources that it is about
to remap. It also adds a call to xc_assign_device to xend and removes
the PRIVILEGED_STUBDOMS flags.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff --git a/Config.mk b/Config.mk
index 79029a0be954c89ef3e95abef64a96d262659e31..63ae38c7b658fa6532ae004270609f9438e054e9 100644 (file)
--- a/Config.mk
+++ b/Config.mk
@@ -150,9 +150,9 @@ QEMU_REMOTE=http://xenbits.xensource.com/git-http/qemu-xen-unstable.git
 # CONFIG_QEMU ?= ../qemu-xen.git
 CONFIG_QEMU ?= $(QEMU_REMOTE)
 
-QEMU_TAG ?= a3285ff385d2568f0226f15fee2b9808ec3b6deb
-# Tue Oct 20 15:16:34 2009 +0100
-# usb hotplug in qemu-dm via xm
+QEMU_TAG ?= b4bb8b3f09d1c873f522f6aebe1f125a6d1854d0
+# Wed Oct 21 16:42:15 2009 +0100
+# passthrough: fix security issue with stubdoms
 
 OCAML_XENSTORED_REPO=http://xenbits.xensource.com/ext/xen-ocaml-tools.hg
 

diff --git a/tools/python/xen/xend/server/pciif.py b/tools/python/xen/xend/server/pciif.py
index 0e54ae14df89e7060081576e936cd6461b675e0d..06b2ec931095df59d07dc02292d66010fcd7df6a 100644 (file)
--- a/tools/python/xen/xend/server/pciif.py
+++ b/tools/python/xen/xend/server/pciif.py
@@ -444,7 +444,15 @@ class PciController(DevController):
         # For hvm guest, (from c/s 19679 on) assigning device statically and
         # dynamically both go through reconfigureDevice(), so HERE the
         # setupOneDevice() is not necessary.
-        if not self.vm.info.is_hvm():
+        if self.vm.info.is_hvm():
+            for pci_dev in pci_dev_list:
+                # Setup IOMMU device assignment
+                bdf = xc.assign_device(self.getDomid(), pci_dict_to_xc_str(pci_dev))
+                pci_str = pci_dict_to_bdf_str(pci_dev)
+                if bdf > 0:
+                    raise VmError("Failed to assign device to IOMMU (%s)" % pci_str)
+                log.debug("pci: assign device %s" % pci_str)
+        else :
             for d in pci_dev_list:
                 self.setupOneDevice(d)
         wPath = '/local/domain/0/backend/pci/%u/0/aerState' % (self.getDomid())

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 731e0cc88c9a5528f85546fddcfa10269f192f97..10847182ae8c48fd7613827f405c5e051d3de969 100644 (file)
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -11,9 +11,6 @@ crash_debug   ?= n
 gdbsx         ?= n
 frame_pointer ?= n
 
-# Allow some delicate passthrough related hypercalls to be made from a stubdom
-privileged_stubdoms ?= y
-
 XEN_ROOT=$(BASEDIR)/..
 include $(XEN_ROOT)/Config.mk
 
@@ -56,7 +53,6 @@ CFLAGS-$(perfc)         += -DPERF_COUNTERS
 CFLAGS-$(perfc_arrays)  += -DPERF_ARRAYS
 CFLAGS-$(lock_profile)  += -DLOCK_PROFILE
 CFLAGS-$(frame_pointer) += -fno-omit-frame-pointer -DCONFIG_FRAME_POINTER
-CFLAGS-$(privileged_stubdoms) += -DPRIVILEGED_STUBDOMS
 CFLAGS-$(gdbsx)         += -DXEN_GDBSX_CONFIG
 
 ifneq ($(max_phys_cpus),)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 581fcb3295c10cab70a4776ecf18d92525b0f8d5..52f3945f06f44ac842287fe717d67e8bb505863b 100644 (file)
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -796,6 +796,11 @@ long arch_do_domctl(
         if ( ret )
             goto bind_out;
 
+        ret = -EPERM;
+        if ( !IS_PRIV(current->domain) &&
+             !irq_access_permitted(current->domain, bind->machine_irq) )
+            goto bind_out;
+
         ret = -ESRCH;
         if ( iommu_enabled )
         {
@@ -820,6 +825,12 @@ long arch_do_domctl(
         if ( (d = rcu_lock_domain_by_id(domctl->domain)) == NULL )
             break;
         bind = &(domctl->u.bind_pt_irq);
+
+        ret = -EPERM;
+        if ( !IS_PRIV(current->domain) &&
+             !irq_access_permitted(current->domain, bind->machine_irq) )
+            goto bind_out;
+
         if ( iommu_enabled )
         {
             spin_lock(&pcidevs_lock);
@@ -848,6 +859,11 @@ long arch_do_domctl(
         if ( unlikely((d = rcu_lock_domain_by_id(domctl->domain)) == NULL) )
             break;
 
+        ret = -EPERM;
+        if ( !IS_PRIV(current->domain) &&
+             !iomem_access_permitted(current->domain, mfn, mfn + nr_mfns - 1) )
+            break;
+
         ret=0;
         if ( domctl->u.memory_mapping.add_mapping )
         {
@@ -895,6 +911,11 @@ long arch_do_domctl(
             break;
         }
 
+        ret = -EPERM;
+        if ( !IS_PRIV(current->domain) &&
+             !ioports_access_permitted(current->domain, fmp, fmp + np - 1) )
+            break;
+
         ret = -ESRCH;
         if ( unlikely((d = rcu_lock_domain_by_id(domctl->domain)) == NULL) )
             break;

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 510be8db125aa944684b3e92649472b87fd8d5d3..74d096f462c66e4e50873164d4a65a4ff0ef69ee 100644 (file)
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -1343,7 +1343,9 @@ int map_domain_pirq(
     ASSERT(spin_is_locked(&pcidevs_lock));
     ASSERT(spin_is_locked(&d->event_lock));
 
-    if ( !STUBDOM_IS_PRIV_FOR(current->domain, d) )
+    if ( !IS_PRIV(current->domain) &&
+         !(IS_PRIV_FOR(current->domain, d) &&
+          irq_access_permitted(current->domain, pirq)))
         return -EPERM;
 
     if ( pirq < 0 || pirq >= d->nr_pirqs || irq < 0 || irq >= nr_irqs )

diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
index 0c284e84bcdb0c3631b397641cd084e1a909c28d..2d65fa4df435ab10e07139688d860be0de8ee34f 100644 (file)
--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -45,7 +45,7 @@ static int physdev_map_pirq(struct physdev_map_pirq *map)
     if ( d == NULL )
         return -ESRCH;
 
-    if ( !STUBDOM_IS_PRIV_FOR(current->domain, d) )
+    if ( !IS_PRIV_FOR(current->domain, d) )
     {
         ret = -EPERM;
         goto free_domain;
@@ -169,7 +169,7 @@ static int physdev_unmap_pirq(struct physdev_unmap_pirq *unmap)
         return -ESRCH;
 
     ret = -EPERM;
-    if ( !STUBDOM_IS_PRIV_FOR(current->domain, d) )
+    if ( !IS_PRIV_FOR(current->domain, d) )
         goto free_domain;
 
     spin_lock(&pcidevs_lock);

diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 0b9ad758ddda0a114431a40d1a9403ee5f845d3d..290b9494ddc35dccac46594312bfc565a5ff3010 100644 (file)
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -231,14 +231,12 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
     case XEN_DOMCTL_ioport_mapping:
     case XEN_DOMCTL_memory_mapping:
     case XEN_DOMCTL_bind_pt_irq:
-    case XEN_DOMCTL_unbind_pt_irq:
-    case XEN_DOMCTL_assign_device:
-    case XEN_DOMCTL_deassign_device: {
+    case XEN_DOMCTL_unbind_pt_irq: {
         struct domain *d;
         bool_t is_priv = IS_PRIV(current->domain);
         if ( !is_priv && ((d = rcu_lock_domain_by_id(op->domain)) != NULL) )
         {
-            is_priv = STUBDOM_IS_PRIV_FOR(current->domain, d);
+            is_priv = IS_PRIV_FOR(current->domain, d);
             rcu_unlock_domain(d);
         }
         if ( !is_priv )

diff --git a/xen/include/xen/config.h b/xen/include/xen/config.h
index 43d6bc6cff249ec3fa860b3eda381ca16a7410f6..7872f13e8d93e88eb3486155aefd1a65bfb63bd0 100644 (file)
--- a/xen/include/xen/config.h
+++ b/xen/include/xen/config.h
@@ -95,10 +95,4 @@ int current_domain_id(void);
 #define __cpuinitdata
 #define __cpuinit
 
-#ifdef PRIVILEGED_STUBDOMS
-#define STUBDOM_IS_PRIV_FOR(x,y) IS_PRIV_FOR(x,y)
-#else
-#define STUBDOM_IS_PRIV_FOR(x,y) IS_PRIV(x)
-#endif
-
 #endif /* __XEN_CONFIG_H__ */